Add option to keep comments in output #124
Conversation
There was a problem hiding this comment.
Thanks for these changes. Overall I like PSR-2 (except for the spaces-instead-of-tabs requirement, but you reverted that so that’s good).
I think outputting comments is a useful addition and I will gladly merge it. If you could implement some of the suggestions I commented, that’d be great but only so long as you find them useful.
| '', | ||
| array_map( | ||
| function (Comment $comment) use ($oOutputFormat) { | ||
| return '/*' . $comment->getComment() . "*/\n"; |
There was a problem hiding this comment.
There is already a render method on Comment that does this. I think you can change that to respect the KeepComments option and then you could replace this whole block with
$sResult .= $oOutputFormat->implode('', $this->getCommentsBefore());| @@ -170,4 +199,21 @@ public function setComments(array $aComments) { | |||
| $this->aComments = $aComments; | |||
| } | |||
|
|
|||
| /** | |||
| * Returns all comments that were declared before this Rule | |||
There was a problem hiding this comment.
replace “Rule” with “RuleSet”
| * Returns all comments that were declared before this Rule | ||
| * @return Comment[] | ||
| */ | ||
| public function getCommentsBefore() |
There was a problem hiding this comment.
I’m not up-to-speed with the comment parsing routines but the way I believe this should work is that the comments associated with this RuleSet would always appear before. If a comment appeared inside, it would be associated with a Rule. And if it were to appear below the RuleSet, it would be parsed with the next RuleSet.
I’m not sure, though, how comments are associated that appear in an otherwise empty document or after the last RuleSet. IMHO, they should be stored on the Document, not simply discarded.
I guess my question is: why is this method needed, can’t we simply output all comments?
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function render(OutputFormat $oOutputFormat) { |
There was a problem hiding this comment.
A Rule should also output its comments. Maybe we should be able to configure in OutputFormat which comments we want to output: only document-level, document-and-ruleset-level, document,-ruleset-and-rule-level.
At @PrestaShop we want to use this library to help us programmatically transform a bunch of CSS files into right-to-left. Our problem is, the license block included in our CSS files was being lost after the process.
Unless I'm mistaken, the Parser is able to read comments into the syntax tree, but they are always ignored when rendering the output CSS.
The main goal of this change is to make it possible to keep comments in the output CSS, at least the ones located at root level of the original CSS.
With that in mind, this PR adds an option
setKeepCommentsinOutputFormatwhich allows toggling this feature on and off (off by default)/Code is mostly whitespace and comments. I followed PSR-2 on some classes, sorry. Feel free to change it back to your standards if you decide to merge it.
For a clutter-free review I suggest looking at 9e23f77, which contains the main change.